Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

generator: Mark QNX c_void types as opaque to get raw pointers #950

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

MarijnS95
Copy link
Collaborator

@MarijnS95 MarijnS95 commented Oct 3, 2024

Fixes #949

Long ago in #339 I added "opaque" types to ensure that pointers to it stay as pointers in FFI rather than get converted to &mut borrows which don't carry the load of an arbitrary pointer value to "something unknown" (= opaque). When the QNX types were added in #429 and #760 some time later, they weren't added to this list and turned into borrows making them impossible to be handled safely in user code.

Note that only SECURITY_ATTRIBUTES remains as a borrow of a c_void, which should probably be changed in some way too. If not in the least because the raw pointer is allowed to be NULL.

Long ago in #339 I added "opaque" types to ensure that pointers to it
stay as pointers in FFI rather than get converted to `&mut` borrows
which don't carry the load of an arbitrary pointer value to "something
unknown" (= opaque).  When the QNX types were added in #429 and #760
some time later, they weren't added to this list and turned into borrows
making them impossible to be handled safely in user code.
@Ralith
Copy link
Collaborator

Ralith commented Oct 3, 2024

making them impossible to be handled safely in user code.

They can't be handled safely regardless, I think, but at least we can have useful setters.

@MarijnS95
Copy link
Collaborator Author

@Ralith wrong wording, I think I meant UB. There are likely more safety constraints on &mut c_void (that a cast/transmute can never uphold) than there are for passing an opaque *mut c_void around?

@Ralith
Copy link
Collaborator

Ralith commented Oct 3, 2024

I think it's actually okay to synthesize references to ZSTs? But the lifetime isn't doing any good here.

@MarijnS95
Copy link
Collaborator Author

Makes sense. What is your verdict on SECURITY_ATTRIBUTES, before we merge this?

@Ralith
Copy link
Collaborator

Ralith commented Oct 3, 2024

That's a tricky case, since unlike a windowing handle it seems like it theoretically could address a value on the stack and readily benefit from lifetime checking. So long as we're defining it ourselves/as c_void, that's difficult but not strictly impossible for a user to take advantage of--typically going through pointer casts will erase lifetime information, but you can put it back if you really want. Do we know where they come from, and in what form?

@MarijnS95
Copy link
Collaborator Author

It's just a POD but with a pointer to a "security descriptor". Looks like the caller is expected to create it as follows:

https://learn.microsoft.com/en-us/windows/win32/secauthz/creating-a-security-descriptor-for-a-new-object-in-c--

Either way it's allowed to be NULL but that should be representable by leaving the raw/FFI pointer field at null_mut() even though our builder has no way to "unassign" it.

@MarijnS95
Copy link
Collaborator Author

API-wise, I wonder if what we're doing here is a semver break since the target type remains the same, and the original &'a mut coerces into *mut.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ScreenSurfaceCreateInfoQNX builder takes c_void handles by reference instead of pointer
2 participants